Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more validation for posting maintenance tasks #2757

Merged

Conversation

johannaengland
Copy link
Contributor

Catches posting maintenance tasks with invalid netbox keys as well as posting maintenance tasks with end time before start time.

Also adds tests for those cases.

Copy link

codecov bot commented Nov 22, 2023

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (fbf4cfd) 55.66% compared to head (dba746a) 56.04%.

Files Patch % Lines
python/nav/web/maintenance/utils.py 55.55% 12 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2757      +/-   ##
==========================================
+ Coverage   55.66%   56.04%   +0.37%     
==========================================
  Files         567      567              
  Lines       41224    41250      +26     
==========================================
+ Hits        22946    23117     +171     
+ Misses      18278    18133     -145     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

github-actions bot commented Nov 22, 2023

Test results

     12 files       12 suites   11m 40s ⏱️
3 291 tests 3 291 ✔️ 0 💤 0
9 348 runs  9 348 ✔️ 0 💤 0

Results for commit dba746a.

♻️ This comment has been updated with latest results.

python/nav/web/maintenance/views.py Outdated Show resolved Hide resolved
@johannaengland
Copy link
Contributor Author

I have now updated it to handle both component keys that are not of type int and handling of given keys where there are no components for those keys.

Currently it still shows at "Selected components" an entry with a "(Component was deleted)", if I should also remove that, let me know, I just wanted to get this out now.

Copy link
Member

@lunkwill42 lunkwill42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should really make an issue out of adding proper form handling to this (I believe this code predates Django in NAV, which is why it's so weird).

I have only one nitpick with your solution (which looks much better now, given the circumstances):

The error messages are "tech-speak-laden", i.e. at minimum, the term pks needs to be expanded to either "primary keys" or perhaps just "identifiers".

Copy link
Member

@lunkwill42 lunkwill42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

squash squash squash ⏩

@johannaengland johannaengland force-pushed the bugfix/maintenance-validation branch from 508428d to dba746a Compare November 24, 2023 09:13
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@johannaengland johannaengland merged commit 3044f3c into Uninett:master Nov 24, 2023
11 checks passed
@johannaengland johannaengland deleted the bugfix/maintenance-validation branch November 24, 2023 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants